-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix:gui spam #133
fix:gui spam #133
Conversation
do not spam the bus with GUI messages if disabled in config
WalkthroughThe changes introduce a new property, Changes
Sequence Diagram(s)sequenceDiagram
participant GUIInterface
participant Methods
GUIInterface->>Methods: Check gui_disabled
alt GUI is disabled
Methods-->>GUIInterface: Return early
else GUI is enabled
Methods-->>GUIInterface: Proceed with GUI actions
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- ovos_bus_client/apis/gui.py (7 hunks)
🔇 Additional comments (3)
ovos_bus_client/apis/gui.py (3)
122-124
: LGTM! Well-implemented property for GUI state checking.The property is correctly implemented with proper typing and follows the configuration structure.
283-284
: LGTM! Consistent implementation of GUI state checks.The early returns when GUI is disabled are consistently implemented across all GUI-related methods, effectively preventing unnecessary message bus traffic when the GUI is disabled.
Also applies to: 331-332, 348-349, 455-456, 485-486, 506-507
122-124
: Verify complete message prevention when GUI is disabled.Let's verify that no GUI-related messages can be emitted when GUI is disabled.
✅ Verification successful
Let me gather more context about the GUI class implementation to see how these emissions are controlled.
Let me try a different approach to check the GUI class implementation.
Let me check specifically how the gui_disabled property is used in method implementations.
GUI message prevention is properly implemented
The verification shows that all GUI-related message emissions are properly guarded by the
gui_disabled
check. The following methods include the guard:
_sync_data
clear()
send_event()
show_page()
remove_page()
remove_pages()
Each method checks
if self.gui_disabled: return
before emitting any messages, effectively preventing all GUI-related message bus traffic when GUI is disabled.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any message emissions that might bypass the gui_disabled check rg -A 3 'gui\.' | rg -B 3 'emit' | rg -v 'gui_disabled'Length of output: 2660
Script:
#!/bin/bash # Check the GUI class implementation and method definitions ast-grep --pattern 'class GUI { $$$ }'Length of output: 42
Script:
#!/bin/bash # Check the GUI class and its methods, particularly focusing on how gui_disabled is used rg -B 3 -A 10 "class GUI" ovos_bus_client/apis/gui.pyLength of output: 1110
Script:
#!/bin/bash # Search for methods that use gui_disabled property rg -B 2 -A 5 "if.*gui_disabled" ovos_bus_client/apis/gui.pyLength of output: 1850
do not spam the bus with GUI messages if disabled in config
Summary by CodeRabbit
New Features
gui_disabled
property to enhance error handling by preventing GUI interactions when the GUI is disabled.Bug Fixes